Feat earthquake report - #521
Conversation
|
🔍 OpenCodeReview found 12 issue(s) in this PR.
|
| builder: (_, state) => ReportDetailPage( | ||
| reportId: state.pathParameters['id']!, | ||
| ), |
There was a problem hiding this comment.
[maintainability · low]
在使用 state.pathParameters['id']! 時,使用了強制解包(bang operator !)。雖然在路由配置中 AppRoutes.earthquakeReportPath 定義為 :id,理論上該參數應該存在,但直接使用 ! 可能在參數缺失或路由解析異常時導致運行時錯誤(Runtime Error)。建議考慮更穩健的處理方式,例如提供預設值或進行檢查。
Suggestion:
| builder: (_, state) => ReportDetailPage( | |
| reportId: state.pathParameters['id']!, | |
| ), | |
| builder: (_, state) => ReportDetailPage( | |
| reportId: state.pathParameters['id'] ?? '', | |
| ), |
| @override | ||
| Future<Result<EarthquakeReport>> get(String id) => guardResult(() async { | ||
| final raw = await _api.getReport(id); | ||
| return EarthquakeReport.fromJson((raw as Map).cast<String, dynamic>()); | ||
| }); |
There was a problem hiding this comment.
[maintainability · low]
在 get 方法中,使用 raw as Map 進行強轉型。若 API 回傳 null 或非 Map 格式,會觸發 TypeError。雖然 guardResult 會捕捉此異常,但建議使用更明確的型別檢查或更穩健的轉換方式,以提高程式碼的預期性與健壯性。
|
|
||
| _EarthquakeReport _$EarthquakeReportFromJson(Map<String, dynamic> json) => | ||
| _EarthquakeReport( | ||
| id: json['id'] as String, |
There was a problem hiding this comment.
[other · low]
reportUrl 與 reportImageUrl 中的 number!.substring(3) 假設了 CWA 編號(serial)至少有 4 個字元。若 API 回傳的 id 第一段小於 4 個字元,將會觸發 RangeError。建議加入長度檢查或使用更安全的方式擷取字串。
Suggestion:
| id: json['id'] as String, | |
| Uri get reportUrl { | |
| final segments = id.split('-')..removeAt(0); | |
| final magCode = (magnitude * 10).floor(); | |
| final numberSuffix = (hasNumber && number!.length > 3) ? number!.substring(3) : ''; | |
| return Uri.parse( | |
| 'https://scweb.cwa.gov.tw/zh-tw/earthquake/details/' | |
| '${segments.join()}$magCode$numberSuffix', | |
| ); | |
| } | |
| /// CWA's rendered report image (地震報告圖). The filename is derived from the | |
| /// Taipei-local origin time, magnitude, and (when numbered) the report's | |
| /// serial suffix — CWA doesn't expose this as a field, only as a static path. | |
| Uri get reportImageUrl { | |
| final t = originTimeUtc.add(const Duration(hours: 8)); // Asia/Taipei | |
| final y = t.year.toString(); | |
| final mo = t.month.toString().padLeft(2, '0'); | |
| final d = t.day.toString().padLeft(2, '0'); | |
| final h = t.hour.toString().padLeft(2, '0'); | |
| final mi = t.minute.toString().padLeft(2, '0'); | |
| final s = t.second.toString().padLeft(2, '0'); | |
| final magCode = (magnitude * 10).floor(); | |
| final numberSuffix = (hasNumber && number!.length > 3) ? number!.substring(3) : ''; | |
| final name = '$y$mo$d$h$mi$s$magCode${numberSuffix}_H.png'; | |
| final yearMonth = name.substring(0, 6); | |
| return Uri.parse('https://scweb.cwa.gov.tw/webdata/OLDEQ/$yearMonth/$name'); | |
| } |
| (k, e) => | ||
| MapEntry(k, AreaIntensity.fromJson(e as Map<String, dynamic>)), | ||
| ), | ||
| time: (json['time'] as num).toInt(), |
There was a problem hiding this comment.
[test · medium]
測試案例中的 reportImageUrl 預期值與實作邏輯不符。實作中透過 originTimeUtc.add(const Duration(hours: 8)) 將時間轉換為台北時間,但測試案例卻預期使用 UTC 時間 (005836)。這將導致單元測試失敗。應將測試預期值修正為台北時間 (085836)。
Suggestion:
| time: (json['time'] as num).toInt(), | |
| test('reportUrl and reportImageUrl are derived from id/time/magnitude', () { | |
| final report = EarthquakeReport.fromJson(json); | |
| expect( | |
| report.reportUrl.toString(), | |
| 'https://scweb.cwa.gov.tw/zh-tw/earthquake/details/2026073100583647053', | |
| ); | |
| expect( | |
| report.reportImageUrl.toString(), | |
| 'https://scweb.cwa.gov.tw/webdata/OLDEQ/202607/2026073108583647053_H.png', | |
| ); | |
| }); |
| @override | ||
| String get reportDetailTitle => 'Earthquake Report'; | ||
|
|
||
| @override | ||
| String reportDetailNumbered(String number) { | ||
| return 'No. $number Significant Earthquake'; | ||
| } |
There was a problem hiding this comment.
[maintainability · high]
此檔案位於 lib/l10n/gen/ 目錄下,通常是由 Flutter 的 localization 工具(如 gen-l10n)根據 .arb 檔案自動生成的。直接手動修改此類生成的檔案會導致下次執行生成指令時,這些修改被覆蓋並遺失。
建議做法:
- 找到專案中的
.arb檔案(例如lib/l10n/app_en.arb)。 - 將這些翻譯內容與參數化邏輯(例如
"reportDetailNumbered": "No. {number} Significant Earthquake")新增到.arb檔案中。 - 重新執行生成指令來更新此
.dart檔案。
| abstract final class MagnitudeColors { | ||
| static const List<(double magnitude, Color color)> _stops = [ | ||
| (2.5, Color(0xFF00C8C8)), | ||
| (3.5, Color(0xFF00C800)), | ||
| (4.5, Color(0xFFFFC800)), | ||
| (6.0, Color(0xFFFF0000)), | ||
| (7.0, Color(0xFF9600FF)), | ||
| ]; | ||
|
|
||
| /// The colour for [magnitude], linearly interpolated between stops and | ||
| /// clamped to the end colours outside the ramp's range. | ||
| static Color of(double magnitude) { | ||
| if (magnitude <= _stops.first.$1) return _stops.first.$2; | ||
| if (magnitude >= _stops.last.$1) return _stops.last.$2; | ||
| for (var i = 0; i < _stops.length - 1; i++) { | ||
| final (lo, loColor) = _stops[i]; | ||
| final (hi, hiColor) = _stops[i + 1]; | ||
| if (magnitude >= lo && magnitude < hi) { | ||
| return Color.lerp(loColor, hiColor, (magnitude - lo) / (hi - lo))!; | ||
| } | ||
| } | ||
| return _stops.first.$2; | ||
| } | ||
| } |
There was a problem hiding this comment.
[test · low]
該工具類別缺乏單元測試。顏色插值邏輯對於地震資訊的視覺呈現至關重要,應確保在邊界值(如震級正好等於 stop 值)、中間值以及極端值下的計算結果皆符合預期。
| } | ||
| return _stops.first.$2; | ||
| } | ||
| } |
There was a problem hiding this comment.
[maintainability · low]
由於函數開頭已處理了超出範圍的情況(magnitude <= _stops.first.$1 與 magnitude >= _stops.last.$1),且迴圈涵蓋了所有範圍,函數末尾的 return _stops.first.$2; 實際上是無法被執行到的死碼 (Dead code)。建議移除以保持程式碼簡潔。
Suggestion:
| } | |
| return _stops.first.$2; | |
| } | |
| } | |
| } | |
| // 這裡理論上不應被執行到,若需符合編譯器要求可拋出異常或返回預設值 | |
| throw UnreachableError(); | |
| } | |
| } |
| import 'package:dpip/app/theme/app_radius.dart'; | ||
| import 'package:flutter/material.dart'; |
There was a problem hiding this comment.
[bug · high]
程式碼中使用了 FontFeature,但未匯入 dart:ui 庫。這會導致編譯錯誤。建議在檔案頂部加入 import 'dart:ui';。
Suggestion:
| import 'package:dpip/app/theme/app_radius.dart'; | |
| import 'package:flutter/material.dart'; | |
| import 'dart:ui'; | |
| import 'package:dpip/app/theme/app_radius.dart'; | |
| import 'package:flutter/material.dart'; |
| // Scale proportionally from the reference 48px badge size so larger/ | ||
| // smaller badges (e.g. the detail header) keep the same visual weight. | ||
| final fontSize = (titleLarge?.fontSize ?? 22) * (size / 48); |
There was a problem hiding this comment.
[maintainability · medium]
fontSize 的計算邏輯 (titleLarge?.fontSize ?? 22) * (size / 48) 缺乏對 size 數值的邊界檢查。如果傳入的 size 為 0 或負數,會導致字體大小異常(例如變成 0 或負數),這可能引發 UI 渲染錯誤或文字完全消失。建議加入對 size 的有效性檢查,例如使用 size.clamp(min_size, max_size) 或確保 size 大於 0。
Suggestion:
| // Scale proportionally from the reference 48px badge size so larger/ | |
| // smaller badges (e.g. the detail header) keep the same visual weight. | |
| final fontSize = (titleLarge?.fontSize ?? 22) * (size / 48); | |
| // Scale proportionally from the reference 48px badge size so larger/ | |
| // smaller badges (e.g. the detail header) keep the same visual weight. | |
| // Ensure size is positive to avoid invalid font size. | |
| final fontSize = (titleLarge?.fontSize ?? 22) * (size / 48).clamp(0.1, double.infinity); |
| test('reportUrl and reportImageUrl are derived from id/time/magnitude', () { | ||
| final report = EarthquakeReport.fromJson(json); | ||
| expect( | ||
| report.reportUrl.toString(), | ||
| 'https://scweb.cwa.gov.tw/zh-tw/earthquake/details/2026073100583647053', | ||
| ); | ||
| expect( | ||
| report.reportImageUrl.toString(), | ||
| 'https://scweb.cwa.gov.tw/webdata/OLDEQ/202607/2026073100583647053_H.png', | ||
| ); | ||
| }); |
There was a problem hiding this comment.
[test · medium]
單元測試對邊界條件與異常資料的覆蓋率不足。目前的測試案例主要針對「快樂路徑」(Happy Path)與單一特殊的 ID 序列(以 000 結尾),缺乏對 API 回傳 JSON 欄位缺失、id 格式錯誤、time 格式異常或 list 結構不完整等異常情況的測試,這可能導致 App 在接收到不完整資料時發生運行時崩潰。建議增加對缺少必要欄位(如 id, mag, time 等)或格式不正確的 JSON 資料的測試案例。
在地震報告 添加詳細資訊